Replaced demo with local instance url in OpenApiDocs#34
Merged
Conversation
Contributor
Author
|
@AltamashShaikh I'll also do a cloud PR that hooks into |
AltamashShaikh
left a comment
AltamashShaikh
left a comment
Contributor
There was a problem hiding this comment.
Looks good, few concerns by AI to check
High-risk issues (blocking)
- The new event dispatch breaks the unit test path that used to work without a container. testReturnsPluginLocalPathsWhenCloudIsDisabled() now constructs a real
PathResolver in tests/Unit/Specs/PathResolverTest.php:26, and getSpecDirectory() immediately calls Piwik::postEvent() through Specs/PathResolver.php:82. Running
vendor/bin/phpunit --bootstrap tests/PHPUnit/proxy/includes.php plugins/OpenApiDocs/tests/Unit/Specs/PathResolverTest.php from the Matomo root errors with
ContainerDoesNotExistException. That is a concrete regression introduced by this refactor.
Low-risk / polish
- Several docblocks in Annotations/AnnotationGenerator.php:857 still describe the behavior as querying demo.matomo.cloud, which is now inaccurate.
- The PathResolver test suite mocks dispatchArtifactBasePathEvent() for most cases but not the default-path test, which is why the regression slipped through. The
tests should consistently isolate external event dispatch.
Contributor
Author
@AltamashShaikh Is this an issue? We just require the full bootstrap.php rather than includes.php. Seems like it's more of an integration test now that we rely on Piwik::postEvent() |
AltamashShaikh
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Removed mentions of demo.matomo.cloud.
Updated path resolver to post an event that allows other plugins to modify the artifact path used, will do a cloud PR to hook into that event
Issue No
Steps to Replicate the Issue
Checklist